Skip to content

Conversation

@Mr-Leshiy
Copy link
Contributor

@Mr-Leshiy Mr-Leshiy commented Nov 7, 2025

Description

Processing scenario when another registration chain could "stole" cip0134uri address, so it become not accessible for the current one after applying such registration.

Related Issue(s)

Part of #578

Description of Changes

  • adding taken_uris: HashSet<Cip0134Uri>, and update_taken_uris for Cip0134UriSet struct. Also modify the original update method, so it removes the taken_uris, which enables the functionality of returning back "stolen" addresses.
  • adding signing_pk_for_role method for the Cip509 type.
  • refactoring new, update methods for the RegistrationChain type.

Please confirm the following checks

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

@Mr-Leshiy Mr-Leshiy added squad: gatekeepers Catalyst App Backend, System Development & Integration Team draft Draft labels Nov 12, 2025
@Mr-Leshiy Mr-Leshiy moved this from New to 🏗 In progress in Catalyst Nov 12, 2025
@Mr-Leshiy Mr-Leshiy added the review me PR is ready for review label Nov 13, 2025
@Mr-Leshiy Mr-Leshiy marked this pull request as ready for review November 13, 2025 12:21
@Mr-Leshiy Mr-Leshiy removed the draft Draft label Nov 13, 2025
Copy link
Member

@stanislav-tkach stanislav-tkach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the change is ok in general, but I don't quite get the idea with the updated URI set. As far as I can see, we are still not storing the full history and only the final state in Cip0134UriSet. In that case why do we want to store "taken" addresses separately? Why not store the resulting final state? For example currently we would have something like this:

x_uris: [A, B, C],
c_uris: [D, E],
taken_uris: [B, C, D],

But it can be represented as

x_uris: [A],
c_uris: [E],

Am I missing something?

@stanislav-tkach
Copy link
Member

Also I would like to see how we handle the caching on the catalyst-gateway side with the updated API.

@github-actions
Copy link
Contributor

📚 Docs Preview

The docs for this PR can be previewed at the following URL:

https://docs.dev.projectcatalyst.io/libs/feat/rbac-stake-address-processing

@github-actions
Copy link
Contributor

github-actions bot commented Nov 17, 2025

Test Report | ${\color{lightgreen}Pass: 554/554}$ | ${\color{red}Fail: 0/554}$ |

Comment on lines +508 to +515
cip509.report().functional_validation(
&format!(
"Trying to apply the first registration to the associated {} again",
cat_id.as_short_id()
),
"It isn't allowed to submit first registration twice",
);
return None;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed that before, but I missed one case and this actually allowed. We can only get a Catalyst ID of a registration if the role 0 certificate is present. That should always be true for the root registration, but it is also allowed for any following registration too.

@Mr-Leshiy Mr-Leshiy merged commit 5c0b665 into feat/rbac Nov 19, 2025
24 checks passed
@Mr-Leshiy Mr-Leshiy deleted the feat/rbac-stake-address-processing branch November 19, 2025 12:51
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Catalyst Nov 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review me PR is ready for review squad: gatekeepers Catalyst App Backend, System Development & Integration Team

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

3 participants